Skip to content

fix(presentationcore): RequerySuggestedEventManager uses wrong typeof key (regression from upstream PR #10715)#2

Merged
oysteinkrog merged 3 commits intoif/mainfrom
fix/revert-typeof-cache-removal-pr10715
May 1, 2026
Merged

fix(presentationcore): RequerySuggestedEventManager uses wrong typeof key (regression from upstream PR #10715)#2
oysteinkrog merged 3 commits intoif/mainfrom
fix/revert-typeof-cache-removal-pr10715

Conversation

@oysteinkrog
Copy link
Copy Markdown
Member

Summary

Fixes an InvalidCastException at runtime under XAML parsing whenever a control with a Command binding gets templated. Root cause: upstream PR dotnet#10715 (cherry-picked here as 87befda) inlined a typeof() call with the wrong class, so two sibling `WeakEventManager` subclasses now register at the same dictionary key. Whichever wins the race makes the other one's cast throw.

Also adds a `commit_denylist` mechanism so the same upstream commit cannot be re-applied accidentally.

The bug

In `CommandManager.cs`:

```csharp
// Before (correct):
Type managerType = typeof(RequerySuggestedEventManager);
RequerySuggestedEventManager manager = (RequerySuggestedEventManager)GetCurrentManager(managerType);
...
SetCurrentManager(managerType, manager);

// After upstream PR dotnet#10715 (broken):
RequerySuggestedEventManager manager = (RequerySuggestedEventManager)GetCurrentManager(typeof(CanExecuteChangedEventManager));
...
SetCurrentManager(typeof(CanExecuteChangedEventManager), manager);
```

`CanExecuteChangedEventManager` is a public class in the same file pair; it already used `typeof(CanExecuteChangedEventManager)` for its own key. After PR dotnet#10715, both managers register at `typeof(CanExecuteChangedEventManager)`. Whichever is constructed first wins; the loser's `CurrentManager` getter retrieves the wrong instance and the cast throws `InvalidCastException: Unable to cast object of type 'CanExecuteChangedEventManager' to type 'RequerySuggestedEventManager'`.

Repro stack trace (real):
```
at System.Windows.Input.CommandManager.RequerySuggestedEventManager.get_CurrentManager()
at System.Windows.Input.CommandManager.add_RequerySuggested(EventHandler)
at WpfFramework.VM.DelegateCommand.add_CanExecuteChanged(EventHandler)
at System.Windows.Input.CanExecuteChangedEventManager.HandlerSink..ctor(...)
at System.Windows.Input.CanExecuteChangedEventManager.AddHandler(...)
at System.Windows.Controls.Primitives.ButtonBase.HookCommand(ICommand)
at System.Windows.Controls.Primitives.ButtonBase.OnCommandChanged(...)
```

The bug was latent on JIT-only builds (the SO discussed in #1 happened first) but became reproducible on R2R'd builds where the static-init order pinned `CanExecuteChangedEventManager` as the first registrant.

What changed

`src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Command/CommandManager.cs` — restore the key to `typeof(RequerySuggestedEventManager)` (its own type) on both `Get` and `Set` calls. Inline form preserved (no need to reintroduce the `managerType` local; that style choice is a separate concern).

`CanExecuteChangedEventManager.cs` is intentionally left unchanged. Its own keys were always `typeof(CanExecuteChangedEventManager)` and that's still the correct identity for it.

commit_denylist mechanism

To prevent this commit from being re-cherry-picked (the cherry-pick automation pulls h3xds1nz PRs in batches):

  • `.if-fork/config.yaml` — new `commit_denylist` section, with upstream commit `0e965e795...` entered as the first denied entry (PR ref + reason).
  • `tools/config-schema.json` — schema for the new section.
  • `tools/check-commit-denylist.py` — hard-gate checker. Walks `(cherry picked from commit ...)` trailers up the chain, so a re-pick of any descendant of a denied commit is also refused. Returns exit 2 + JSON verdict.
  • `.if-fork/prompts/cherry-pick.md` — new Step 2.5 calls the checker before the graduation check; on match, emits a ledger event `refused` with reason `commit_denylist` and exits.

Verification

  • crossgen2-compiled InitialForce.WPF if.63 nupkg consumed by MotionCatalyst → reproduces the cast failure during MainWindow template loading.
  • After this fix (built locally → if.64), the same app starts cleanly past the templating phase.
  • `python tools/check-commit-denylist.py 87befda` → exit 2, verdict `denied`, matched via the upstream SHA in the trailer.
  • Test runs unmodified; no behavior change to `CanExecuteChangedEventManager` paths.

Upstream

Comment will be filed on dotnet#10715 with a sanitized repro so upstream can fix in their tree before this commit propagates further.

🤖 Generated with Claude Code

Claude (Initial Force WPF Bot) and others added 3 commits April 29, 2026 23:30
Stock dotnet/wpf DLLs in Microsoft.WindowsDesktop.App ship with
ReadyToRun native code, baked in by dotnet/runtime's runtime-pack
assembly step. Our fork builds the libraries via build.cmd but does
not run that step, so the DLLs we ship in the InitialForce.WPF nupkg
are JIT-only. This caused stack overflows in consumers: JIT'd frames
are slightly fatter than R2R'd frames, and WPF code paths that are
already deep on the stack (dispatcher unhandled-exception handler ->
MessageDialog.xaml -> BAML -> WPFLocalizeExtension's 800-culture
iteration) overflow the 1 MB thread stack.

Add a workflow step that downloads the upstream
Microsoft.NETCore.App.Crossgen2.win-x64 NuGet package (cached under
.tools-cache/) and runs crossgen2 over the 4 staged DLLs in both
packaging trees (InitialForce.WPF and InitialForce.WPF.RuntimeOverride).
Each output is verified to contain the R2R magic before replacing the
input. --targetarch matches the matrix.arch so we get win-arm64 R2R
images for the arm64 build.

Verified locally: crossgen2 10.0.7 successfully R2R-compiles all 4
patched DLLs (PresentationCore, PresentationFramework, WindowsBase,
System.Xaml). Output sizes grow ~0-145% (varies by symbol density),
all contain the RTR magic at the expected offset, and consuming the
R2R'd DLLs eliminates the deep-stack SO that the previous nupkgs
exhibited.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ySuggestedEventManager

Upstream commit 0e965e7 ("Avoid caching typeof result to decrease code size",
PR dotnet#10715, cherry-picked here as 87befda) inlined typeof() calls
in two related event managers. In CommandManager.RequerySuggestedEventManager
the inlined typeof points at the WRONG class:

  Before:
    Type managerType = typeof(RequerySuggestedEventManager);
    var manager = (RequerySuggestedEventManager)GetCurrentManager(managerType);
    SetCurrentManager(managerType, manager);

  After (broken):
    var manager = (RequerySuggestedEventManager)GetCurrentManager(typeof(CanExecuteChangedEventManager));
    SetCurrentManager(typeof(CanExecuteChangedEventManager), manager);

Both RequerySuggestedEventManager and CanExecuteChangedEventManager now
register at the same dictionary key (typeof(CanExecuteChangedEventManager))
in WeakEventManager's per-thread table. Whichever runs first wins. The
loser's CurrentManager getter then casts the wrong instance to its own
type and throws InvalidCastException at runtime — reproducible whenever a
Button (or any FrameworkElement with Command) gets templated through XAML
parsing.

Restore the dictionary key to typeof(RequerySuggestedEventManager) for that
manager. The CanExecuteChangedEventManager.cs path is left unchanged: its
key was already typeof(CanExecuteChangedEventManager) before and after, so
that file does not need touching here.

Also adds a commit_denylist mechanism so this regression cannot be re-applied:

  - .if-fork/config.yaml: new commit_denylist section, with 0e965e7 entered
    as the first denied commit (with PR ref and reason).
  - tools/config-schema.json: schema for the new section.
  - tools/check-commit-denylist.py: hard-gate checker invoked by cherry-pick
    automation. Walks (cherry picked from commit ...) trailers up the chain
    so a re-pick of any descendant is also refused.
  - .if-fork/prompts/cherry-pick.md: new Step 2.5 calls the checker before
    Step 3 (graduation check), refuses with ledger event "refused" + reason
    "commit_denylist" if matched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@oysteinkrog oysteinkrog force-pushed the fix/revert-typeof-cache-removal-pr10715 branch from eb84b3b to 9e9bdd6 Compare April 30, 2026 13:20
@oysteinkrog oysteinkrog merged commit a105e10 into if/main May 1, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant